set maxRequestsPerHost of okhttp to support slow response requests and high TPS#708
Merged
Conversation
mukundansundar
left a comment
mukundansundar
left a comment
Contributor
There was a problem hiding this comment.
@skyao Keep in mind that in future we will be moving away from OkHttp and move to native Java Client. But I believe these parameters will still add value there.
skyao
changed the title
Member
Author
OK. Before we changed to native java client, we still need OkHttp. The fix for default maxRequestPerHost is very important. |
artursouza
previously approved these changes
Mar 18, 2022
Contributor
|
@skyao Please, sign off the commits. This is a new requirement in our repos since the CNCF donation. |
mukundansundar
previously approved these changes
Mar 18, 2022
…d high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com>
Signed-off-by: Sky Ao <aoxiaojian@gmail.com>
Signed-off-by: Sky Ao <aoxiaojian@gmail.com>
Signed-off-by: Sky Ao <aoxiaojian@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #708 +/- ##
============================================
+ Coverage 80.66% 80.72% +0.06%
Complexity 993 993
============================================
Files 81 81
Lines 3010 3020 +10
Branches 340 340
============================================
+ Hits 2428 2438 +10
Misses 416 416
Partials 166 166
Continue to review full report at Codecov.
|
fackee
pushed a commit
to fackee/java-sdk
that referenced
this pull request
Mar 28, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee
pushed a commit
to fackee/java-sdk
that referenced
this pull request
Mar 28, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee
pushed a commit
to fackee/java-sdk
that referenced
this pull request
Mar 28, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee
pushed a commit
to fackee/java-sdk
that referenced
this pull request
Mar 28, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee
pushed a commit
to fackee/java-sdk
that referenced
this pull request
Mar 28, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
artursouza
pushed a commit
to pkedy/java-sdk
that referenced
this pull request
Jun 22, 2022
…d high TPS (dapr#708) * set maxRequestsPerHost of okjava to support slow response requests and high TPS Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * fix typo in comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update comments Signed-off-by: Sky Ao <aoxiaojian@gmail.com> * update javadoc Signed-off-by: Sky Ao <aoxiaojian@gmail.com> Signed-off-by: Artur Souza <artursouza.ms@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In dapr java sdk, there are some default settings in okhttp which we don't set and use the default value:
The default value 5 of maxRequestsPerHost is totally unacceptable, it will lead to very serious problems of very low throughput as discribed in issue #709.
And the default value 64 of maxRequests is not enough for the user case of high TPS. For example, if the response time is 100 ms, 64 connections leads to at most 640 TPS.
Issue reference
This PR is to fix this issue.
#709
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: